fix(rendering): make SpriteEcsComponent.pivot Y-up to match the rest of the engine - #590
Conversation
…of the engine pivot used a Y-down convention ((0, 0) was the sprite's top-left) while world position, rotation, and every other Y-facing value are Y-up. The mismatch was an internal projection detail (the shader flips Y) leaking through pivot's public API, invisible at the default centered pivot. Flips pivot's Y in sprite.vert.glsl before it's combined with the quad's local position, and updates computeNineSliceRegions' pivot handling to match so nine-slice region placement agrees with the corrected shader. Breaking change for content using a non-centered pivot. Fixes #585
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
In the ECS demo. The position looks correct, but the rotation looks inverted? |
|
Looked into this — this PR doesn't touch rotation at all, and for a centered pivot (which the ECS demo's To double-check rather than just reason about it, I tracked the star's actual on-screen rotation with a frame-diff (rotate-and-correlate) measurement against two captured frames ~520ms apart on this branch's build: the star rotates ~30.5° counter-clockwise on screen in that window, matching So the rotation direction looks correct to me, and unrelated to this PR either way — happy to be pointed at a specific frame/timestamp if it still looks inverted to you, or if you meant something else by "rotation" (e.g. the orbit path direction, which is independent of the sprite's own spin and also unchanged by this diff). Generated by Claude Code |
…ivot fix dev's #590 flipped SpriteEcsComponent.pivot to Y-up ((0, 0) bottom-left, (1, 1) top-right) and fixed computeNineSliceRegions to match, but shapeText has its own separate pivot math that was explicitly modeled on nine-slice's old (Y-down) formula. That's now inconsistent with the rest of the engine, including the sprite instancing pipeline text's own glyph quads render through. Applies the same fix nine-slice got: pivot.y's contribution flips via (1 - pivot.y) instead of pivot.y directly. Only affects a non-centered pivot; the default (0.5, 0.5) is unaffected. Updates the pivot tests and docs to match. Also merges dev in to pick up #590 and everything else merged since this branch was created. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0113gCpCEDFQad8Y9dJuahSq
2 participants
Summary
SpriteEcsComponent.pivotused a Y-down convention ((0, 0)was the sprite's top-left) while world position, rotation, and every other Y-facing value in the engine are Y-up. The mismatch was an internal projection detail (the vertex shader flips Y before the projection matrix flips it back forposition/rotation) leaking through pivot's public API - invisible at the default centered pivot, which is why it survived.Traced the full pipeline per the issue and confirmed the analysis:
bindSpriteInstanceDatapre-negatesposition.world.yandrotation.worldto compensate for the projection's Y flip, butsprite.vert.glslcomputes the pivot-based local offset after that compensation, so it never gets flipped -pivot.y = 0landed at the sprite's top instead of its bottom.Changes
sprite.vert.glsl: negatea_instancePivot.yalongside the existing[0,1] -> [-1,1]doubling, so pivot's Y now gets the same flipposition/rotationalready get.(0, 0)is now bottom-left,(1, 1)is top-right.computeNineSliceRegions(compute-nine-slice-regions.ts): updated the region-offset math ((1 - pivot.y) * height - ...instead ofpivot.y * height - ...) so nine-slice region placement agrees with the corrected shader convention. The centered pivot(0.5, 0.5)produces identical output before and after (1 - 0.5 === 0.5), so this is a no-op for every sprite in the repo (no demo or test uses a non-centered pivot).sprite-component.ts: updatedpivot's JSDoc to describe the new Y-up convention.compute-nine-slice-regions.test.ts: replaced the single top-left-pivot test with a parameterized case pinning all four corners (bottom-left, bottom-right, top-left, top-right), per the issue's test-coverage callout.CHANGELOG.md: added aFixedentry under[Unreleased], flagged as a breaking change for non-centered pivots.Related issue(s)
Closes #585
Out of scope
The issue's design doc reference (
design/ui-system.md's pivot bridge, from #580) isn't indevyet, so there's nothing to remove there in this PR.Verification checklist
npm run check-typespasses with 0 errorsnpm testpasses (1036 tests)npm run lintpasses with 0 errors (pre-existing, unrelated TODO warnings only)npm run cspellpasses with 0 errorsnpm run check-exportspassespivot's documented semantics - no export changes needed/documentation-site/docs/docs- no page documents the pivot convention, so nothing to updategrep), so no demo code changes; still built/dist, randocumentation-site'stypecheck/build, and visually checked thenine-sliceandbrick-breakerdemos in a browser to confirm no regression at the default centered pivotChangelog
## [Unreleased]→#### FixedinCHANGELOG.mdGenerated by Claude Code